v1.24.0: step 20's verify used a two-dot diff and reported main's newer files as your deletions - #21
Merged
Merged
Conversation
…n one afternoon
Step 20's rebuild path — `git reset --hard origin/main` + cherry-pick, for when the
feature branch was already squash-merged — ended with
git diff --stat origin/main..HEAD
to confirm only the docs files remained before pushing.
Two dots compare tip to tip. Every file `main` gained after you branched therefore
renders as a deletion you appear to be making. That is right only in the instant of
the hard reset, and wrong the moment any parallel session merges before you push —
which is the exact situation the step exists for. In the repo where this was found
`main` moved four times in two days and the line fired spuriously twice, each time
starting an investigation into a data loss that had not happened.
Verified against GitHub this session: for a real PR, GitHub reported 5 files / 219
insertions / 16 deletions and `git diff --stat <base>...<head>` reproduced it exactly,
while the two-dot form returned a different file set and invented deletions.
Now three dots, plus a second line the step never had:
git diff --diff-filter=D --name-only origin/main...HEAD # must be empty
Three dots tell you what the branch proposes; they do not by themselves make the check
loud about the thing that matters. A branch whose TREE is stale — from `git reset --soft
origin/main`, or an old worktree committed with `git add -A` — passes `git merge-base
--is-ancestor`, reports 0 commits behind, and a fast-forward push then replaces main's
tree wholesale. Reproduced in a fixture repo here: fast-forwardable, 0 behind, and the
deletion gate is the only one of the three checks that sees the 322-line file going
away. In the source repo a PR of exactly that shape deleted a client-facing 322-line
file while describing itself as a copy fix; a follow-up restored it 17 minutes later.
Deliberate hardening, same idea, two more sites: `"$BASE"..HEAD` in step 24b and in
reverse_lint_step.sh. BASE is contractually a SHA on the branch's own history, and where
that holds the two forms are byte-identical — but nothing enforces it, and step 20's own
rebuild destroys the ancestor property outright. Both now use three dots. Three dots need
a fork point to exist, so both also gain a `git merge-base` guard: a BASE from an
unrelated history now reports SKIPPED with its reason rather than letting an erroring
diff read as "nothing changed", which is this skill's own rule about clean and never-ran
not looking alike.
Left alone on purpose: `git log --oneline origin/BRANCH..HEAD` in step 19. In `git log`
two dots mean "commits reachable from B but not A" — exactly what "my unpushed commits"
wants. Three dots there would be the symmetric difference and wrong.
Verification: `npm test` green — 79 tests, 0 fail, 2 pre-existing skips (no
eval-suite.json). The suite executes reverse_lint_step.sh end to end against fixture
HOMEs. Also checked by hand: ancestor BASE scans as before, orphan BASE hits the new
guard, literal HEAD~N still hits the old one, `sh -n` clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JfM4bKm5Y8F8KqFB6xjKFP
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The wrong line
plugins/session-handoff/SKILL.md, step 20, the rebuild path for a branch that was already squash-merged (git reset --hard origin/main+ cherry-pick the docs commits). It ended with:Two dots compare tip to tip. Everything
maingained after you branched therefore renders as a deletion you appear to be making. Two dots are correct only in the instant of the hard reset, and wrong the moment any parallel session merges before you push — which is exactly the situation the step exists for. In the repo where this was found,mainmoved four times in two days and this line fired spuriously twice, each time starting an investigation into a data loss that had not happened; one of them got a whole exploration prompt written for it.Verified against GitHub: for a real PR, GitHub reported 5 files / 219 insertions / 16 deletions, and
git diff --stat <base>...<head>reproduced that exactly, while the two-dot form returned a different file set and invented deletions.The fix
Three dots, plus a deletion gate the step never had:
Three dots tell you what the branch proposes; they do not by themselves make the check loud about the thing that matters. The deletion line does. The hazard is real and was reproduced in a fixture repo for this PR: a branch whose tree is stale — from
git reset --soft origin/main, or an old worktree committed withgit add -A— passesgit merge-base --is-ancestor, reports 0 commits behind, and a fast-forward push then replacesmain's tree wholesale. Fixture output:In the source repo, a PR of exactly that shape deleted a client-facing 322-line file from
mainwhile describing itself as a copy fix; a follow-up PR restored it 17 minutes later. The deletion gate is the one command that sees it.Deliberate hardening: the two
$BASE..HEADsitesI checked the BASE-resolution logic rather than guessing, and changed both.
scripts/reverse_lint_step.shhard-SKIPs unlessBASEresolves as a revision, and step 24b's fence does the same — but resolving is not the same as being on this branch's history, nothing enforces the ancestor property, and step 20's own rebuild destroys it outright.BASEis an ancestor (the contracted case),A..BandA...Bare byte-identical. No behaviour change.main's own newer files as though this session had deleted them.So both now use three dots. Three dots need a fork point to exist at all, so both also gain a
git merge-baseguard — aBASEfrom an unrelated history now reports SKIPPED with its reason rather than letting an erroring diff read as "no lessons file changed" / "no SKILL.md was touched". That is this skill's own rule that clean and never ran must not look alike, applied to the one case where three dots could have introduced a silent skip.Left alone on purpose
git log --oneline origin/BRANCH..HEADin step 19. Ingit log, two dots mean "commits reachable from B but not A" — exactly what "my unpushed commits" wants. Three dots there would be the symmetric difference and wrong.Verification
npm testgreen: 79 tests, 0 fail, 2 pre-existing skips (eval-suite.jsonnot present). The suite executesreverse_lint_step.shend to end against fixtureHOMEs, so the script change is covered rather than only grepped.BASEscans as before (clean (1 file(s) scanned)), orphanBASEhits the new guard, literalHEAD~Nstill hits the old one,sh -nclean.git diff --diff-filter=D --name-only origin/main...HEADempty, and the three-dot stat shows only the six intended files.package.json,.claude-plugin/marketplace.json,plugins/session-handoff/.claude-plugin/plugin.json,SKILL.mdfrontmatter, README Version History), which the manifest-consistency tests cross-check.🤖 Generated with Claude Code
https://claude.ai/code/session_01JfM4bKm5Y8F8KqFB6xjKFP